added SecondaryVertex container #132
Conversation
| - float parentDca2PV // parent dca to primary vertex | ||
| - float parentDca2PVError // parent dca_error to primary vertex | ||
| VectorMembers: | ||
| - edm4hep::Vector3f daughterMomentum // daughter track momentum at the decay vertex |
There was a problem hiding this comment.
I like the availability of track state propagated to the decay vertex, but I think only providing the 3-momentum is just a subset of that. In particular, it doesn't have the covariance matrix.
There was a problem hiding this comment.
Wouter, I agree that a full covariance matrix including momentum with positions (DCA positions of the tracks w.r.t the reconstructed vertex) would be more complete. From previous experience to my knowledge, the relevant covariance matrices were used in reconstruction and fitting, but not really used in the downstream analysis. Most of the downstream analysis will work on optimization with the selection cuts on various topology (error-normalized) variables saved here. But if there is an interest to keep them, it shouldn't be difficult to add the full covariance in.
There was a problem hiding this comment.
I think adding that info is the way to go! I understand that right now that only the 3-momenta is really used in analysis, but I think that offering a more complete set of information on the daughters goes a long way towards ensuring that the datatype (or datatypes) implemented here can support a much wider array of analyses in the near(ish) term.
There was a problem hiding this comment.
Thanks for this development, Xin! I think all of the information defined in this type is great, and -- like I mention in the comments below -- I think we can still add more. However, the question to resolve now is: how is this information organized?
Building on what I discussed in EDM4eic#127, I think we can delineate 3 levels of information in this type:
- Vertex-specific info (position, chi2, etc.)
- Particle-specific info (kinematics, mass, etc.)
- And topological info that relates particles to the vertex (DCA, etc.)
All of this is important to have, but combining it into one type runs counter to what we've strived to do in other types. As-is, this would be the one place in the model where if -- for example -- a user wanted kinematic information for a particle (a daughter), they would have to go through a vertex interface rather than through the defined particle/track/cluster interfaces.
While there are no technical barriers to doing so, integrating multiple levels of information like this makes the data model less intuitive and harder to navigate.
Tying all of this together, I have 2 concerns with the type as-is:
- that it deviates from a few precedents we've established with other types;
- and I'm concerned about the extent to which this type could be generalized for algorithms/analyses other than the helix swimmer.
I understand that this type was intended to be a 1st pass at this structure that we would revisit, but I think it's very much worth thinking about this more generally even in this 1st pass. It's easy for us to add types to the model, but it's much more challenging to change them once they're integrated into algorithms and output.
The alternatives I sketched out in EDM4eic#127 could be an approach (but definitely not the only one!) to organizing the info here in such a way that's much more general!
| - int daughterPDG // daughter PDG | ||
| - float daughterDca2PV // daughter dca to primary vertex | ||
| - float daughterDca2PVError // daughter dca_error to primary vertex | ||
| - int daughterPairIndices // track indices for any pair |
There was a problem hiding this comment.
Building on some of the discussion from EDM4eic#127: this is one field that would be made redundant if rather than storing an explicit subset of kinematic info for the daughters, we utilized relations. For example, if we link to the vertex to the daughter particles in a one-to-many relation, then PODIO will auto-generate a branch in the ROOT output to store the indices of linked daughters (the branches prepended with a _).
Not to mention, this also deviates from our current standard practice of preferring PODIO relations over storing specific indices (see here).
| OneToOneRelations: | ||
| - edm4eic::Vertex primaryVertex // associated primary vertex | ||
| OneToManyRelations: | ||
| - edm4eic::ReconstructedParticle associatedParticles // particles associated to this vertex. |
There was a problem hiding this comment.
This, e.g., could be the relevant field to link to the decay products and the mother!
| - float daughterPairDca // dca between any pair of tracks | ||
| - float daughterPairDcaError // dca_error between any pair of tracks | ||
| OneToOneRelations: | ||
| - edm4eic::Vertex primaryVertex // associated primary vertex |
There was a problem hiding this comment.
For my own understanding: is there a reason why it we would always want to link to only the primary vertex?
I'm imagining something like an upsilon decay where we might have an intermediate resonance that decays further, e.g. an upsilon. In which case, the relevant decay length for the intermediary (like a D0) would be between the upsilon vertex and the intermediary's decay vertex...
| - float parentDca2PV // parent dca to primary vertex | ||
| - float parentDca2PVError // parent dca_error to primary vertex | ||
| VectorMembers: | ||
| - edm4hep::Vector3f daughterMomentum // daughter track momentum at the decay vertex |
There was a problem hiding this comment.
I think adding that info is the way to go! I understand that right now that only the 3-momenta is really used in analysis, but I think that offering a more complete set of information on the daughters goes a long way towards ensuring that the datatype (or datatypes) implemented here can support a much wider array of analyses in the near(ish) term.
Briefly, what does this PR introduce?
Add a new container SecondaryVertex to store topological variables used for secondary vertex reconstruction
What kind of change does this PR introduce?
Please check if this PR fulfills the following:
Does this PR introduce breaking changes? What changes might users need to make to their code?
None
Does this PR change default behavior?
None